Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests on React 17 #498

Merged
merged 3 commits into from
Dec 10, 2020
Merged

Add tests on React 17 #498

merged 3 commits into from
Dec 10, 2020

Conversation

rexxars
Copy link
Collaborator

@rexxars rexxars commented Oct 24, 2020

Upgrades the peer dependency version range to support React 17, and uses React 17 for the tests. Switches to using the unofficial @wojtekmaj/enzyme-adapter-react-17 adapter for enzyme, for now (until the official adapter is released).

On a slightly unrelated note - I'm not sure I agree that @types/react should be a peer dependency?

Closes GH-497.

@rexxars rexxars requested a review from wooorm October 24, 2020 12:47
@rexxars
Copy link
Collaborator Author

rexxars commented Oct 24, 2020

Hmm, this seems to be failing with npm 7 because of upstream peer dependencies, too - filed a PR

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rexxars Good to go, I think?

@rexxars
Copy link
Collaborator Author

rexxars commented Oct 25, 2020

@rexxars Good to go, I think?

Yep. The react-katex dependency causes some issues with npm7, but only in development mode. Will create a separate PR to get rid of it and simplify the tests it's used in.

@rexxars
Copy link
Collaborator Author

rexxars commented Oct 25, 2020

Not sure there's much we can do about the tests failing - seems to be because of upstream enzyme adapter peer dependency ranges. Did you want more changes here, @wooorm? Or should we leave it until the official enzyme adapter is out?

@wooorm
Copy link
Member

wooorm commented Oct 28, 2020

An official version seems to get closers: enzymejs/enzyme#2430 (comment).

I’m fine with the changes here and using the other package now, but I’m unhappy with the failing tests 😬 The enzyme PR also mentions a PR to proptypes too, which seems to be coming after the enzyme update.
Can we change the default install in Travis to include the --legacy-peer-deps mention in the npm failure in Travis?

@Hypnosphi
Copy link

@wooorm @rexxars can you please consider just updating peerDependencies range for now?

package.json Outdated Show resolved Hide resolved
wooorm added a commit that referenced this pull request Nov 19, 2020
* Fix to remove 15, which didn’t work anymore: this would normally be
  breaking (and we take deprecating older versions seriously), but as
  it didn’t work before, this change instead catches problems earlier
* Allow React 17 (and later) as a peer dependency: we can’t actually
  add code to test this yet because we’re waiting for our test
  frameworks to update too (see GH-498), but checking locally React 17
  seems supported

Related to: GH-498.
@wooorm
Copy link
Member

wooorm commented Nov 19, 2020

I’ve unlocked the package already, this PR can add tests for it later when enzyme updates.

bb0bdde

@ChristianMurphy
Copy link
Member

A possible way to unblock this would be to migrate from enzyme to testing library react, which supports React 17.
https://testing-library.com/docs/react-testing-library/intro/

@ChristianMurphy
Copy link
Member

@rexxars #519 should unblock the enzyme peer dependency issue, when you have a chance to rebase off main, this should be good to go.

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Dec 10, 2020

Synced with develop, the react-katex test is now causing issues, opened #521 (which targets this branch) to address this

@codecov-io

This comment has been minimized.

@wooorm wooorm changed the title Add support for React 17 Add tests on React 17 Dec 10, 2020
@wooorm wooorm merged commit 634f88d into main Dec 10, 2020
@wooorm wooorm deleted the react-17 branch December 10, 2020 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Update React version to React 17 (peerDependencies)
5 participants